[Datahub]: Fix missing "record not found" message #884
Conversation
|
Affected libs:
|
|
📷 Screenshots are here! |
There was a problem hiding this comment.
Thank you @cmoinier for fixing this issue and especially for adding a e2e test for it.
I'm still not quite sure if we should rely on the presence of the title though. But I don't have another solution atm.
| @@ -70,6 +70,11 @@ export class RecordMetadataComponent { | |||
| mergeMap((uuid) => this.sourceService.getSourceLabel(uuid)) | |||
| ) | |||
|
|
|||
| loadFeedBacks$ = this.metadataViewFacade.metadata$.pipe( | |||
| map((record) => record?.title), | |||
There was a problem hiding this comment.
Not sure if this is a use case, but what if the record doesn't have a title but all the other fields? Is there an error somewhere that we could catch instead of relying on the presence of the title?
There was a problem hiding this comment.
Yeah, I agree that it's not the best. It's just assuming that the title is the field that has the most chances to be filled...
I did try other solutions, such as using "error$" (but there was always at least one type of error that was preventing the feedbacks from loading in a working dataset). Using "isPresent$" or "metadata$" was also useless because there is indeed a metadata (made of uniqueIdentifier and title, even the fake ones).
There was a problem hiding this comment.
Oh, ok I see. Hmm... maybe @jahow has an idea for this?
There was a problem hiding this comment.
I think you should rely on metadataViewFacade.isPresent$ instead :) see above in the HTML, other blocks are using it as well
There was a problem hiding this comment.
I did try to use it, and it didn't work, because isPresent$ is always true when the feedbacks are loading (see second comment in this thread)
There was a problem hiding this comment.
Oh sorry, so much for reading quickly... There might be an issue there because isPresent$ should be precisely used for that.
There was a problem hiding this comment.
Will investigate that then 🙂
There was a problem hiding this comment.
As per the last commit, isPresent works correctly again. It turns out that the "error" state was set to null while the feedbacks were loading. I removed that, and it doesn't seem to break anything.
There was a problem hiding this comment.
Right, well done, this must be because of this reducer: https://github.com/geonetwork/geonetwork-ui/pull/849/files#diff-4a24c2ba947113aa39efacb131d5ce9ba63caee690c409d9d321c55b30cc0b51R83-R87
I think it makes sense that way since the error clearing has to happen only once when the complete metadata starts to load.
There was a problem hiding this comment.
Okay. So is there something left to change on this PR then?
Description
This PR fixes the absence of the "record not found" message.
The issue was indeed linked to the userFeedbacks loading, regardless of the "notFound" error.
It was linked to the fact that userFeedBacks component only needs a metadataUuid to load, and any fake record (eg "abcd") was working with it.
This workaround simply hides the userFeedBack when the metadata does not have a title, since it's not possible to use the error state as long as a metadataUuid exists.
It also adds a e2e test to make sure this error will be caught for now on.
This work is sponsored by [IGN].